Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sending the UTF8 command to the POP3 Server #450

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BenediktS
Copy link

Newer POP3 Server are not delivering the emails correct, if the clients don't tell them that they are UTF8 compatible. https://tools.ietf.org/html/rfc6856.html

This patch sends the UTF8 command, if the server is requesting it.

Newer POP3 Server are not delivering the emails correct, if the clients don't tell them that they are UTF8 compatible.
https://tools.ietf.org/html/rfc6856.html

This patch sends the UTF8 command, if the server is requesting it.
@rlebeau rlebeau added Type: Enhancement Issue is proposing a new feature/enhancement Element: POP3 Issues related to TIdPOP3 and TIdPOP3Server labels Mar 6, 2023
Copy link
Member

@rlebeau rlebeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not enough to just send the UTF8 command, other areas of TIdPOP3's logic would need to be updated to actually handle UTF-8 correctly, per the RFC:

This specification extends POP3 using the POP3 extension mechanism [RFC2449] to permit un-encoded UTF-8 [RFC3629] in headers and bodies (e.g., transferred using 8-bit content-transfer-encoding) as described in "Internationalized Email Headers" [RFC6532]. It also adds a mechanism to support login names and passwords containing a UTF-8 string (see Section 1.1 below), a mechanism to support UTF-8 strings in protocol-level response strings, and the ability to negotiate a language for such response strings.

This specification also adds a new response code to indicate that a message was not delivered because it required UTF-8 mode (as discussed in Section 2) and the server was unable or unwilling to create and deliver a surrogate form of the message as discussed in Section 7 of "IMAP Support for UTF-8" [RFC6855].

Your change does not address any of those points.

At the very least, after sending the UTF8 command and getting back a success response, you would need to set the IOHandler.DefStringEncoding property to IndyTextEncoding_UTF8 to allow subsequent POP3 commands/responses to actually use UTF-8 - which is especially necessary if the USER argument of the server's UTF8 capability is present, in which case you would need to apply SASLprep to USER, PASS and APOP commands (but Indy does not currently implement any support for SASLprep). Then there is a whole issue with supporting UTF-8 with SASL authentications, too.

But also, setting the IOHandler.DefStringEncoding would be needed in order to allow the TIdPOP3.Retrieve() and TIdPOP3.RetrieveHeader() methods to receive unencoded UTF-8 in email headers (the encoding of email bodies is handled internally by TIdMessageClient using 8bit reads and charset decodings, which is behavior that TIdPOP3 can't change).

@@ -587,6 +587,18 @@ procedure TIdPOP3.SetSASLMechanisms(AValue: TIdSASLEntries);
FSASLMechanisms.Assign(AValue);
end;

procedure TIdPOP3.SendUTF8IfAdvertised;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really liking the naming of this method. If you look at other Indy components, they use more general-purpose methods for testing for available capabilties (eg, TIdNNTP.IsExtCmdSupported(), TIdDICT.IsCapaSupported(), TIdFTP.IsExtSupported(), etc). I think your method needs to look a little more like TIdFTP.FindAuthCmd(), for example.

var
Capa : string;
begin
for Capa in FCapabilities do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indy cannot use for..in loops, as it still supports older compilers.

Capa : string;
begin
for Capa in FCapabilities do
if TextStartsWith(Capa, 'UTF8 ') then
Copy link
Member

@rlebeau rlebeau Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UTF8 capability has an optional USER argument. If that argument is not present, the trailing space is likely to also not be present, which would cause this TextStartsWith() call to return False. You can use IdGlobal.Fetch() to retrieve the first word from the Capa string before then comparing it.

@@ -619,6 +631,7 @@ procedure TIdPOP3.Connect;
if FAutoLogin then begin
Login;
end;
SendUTF8IfAdvertised;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sending the UTF8 command at this location is a Catch-22. AutoLogin is True by default, so you would typically be sending the UTF8 command after Login() has already sent the STLS and USER/PASS/APOP/AUTH commands, which is too late for sending UTF-8 encoded authentication. On the other hand, the UTF8 command is not allowed to be sent before an STLS command, so you can't just move the call before Login().

So, you are going to have to move the UTF8 command inside of Login() itself instead, so that Login() can send the UTF8 command after sending the STLS command and before sending the USER/PASS/APOP/AUTH commands. If AutoLogin is False, Connect() should not send UTF8 at all, as that would then prevent a later call to Login() from sending the STLS command.

The alternative would be to manually keep track of whether TIdPOP3 is currently operating in ASCII or UTF8 mode and then send the UTF8 command only when absolutely necessary for a subsequent command to use UTF-8.

@Neustradamus
Copy link

@BenediktS: Have you progressed on your PR?

@rlebeau rlebeau added the Status: Pending Issue is pending external update or release label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Element: POP3 Issues related to TIdPOP3 and TIdPOP3Server Status: Pending Issue is pending external update or release Type: Enhancement Issue is proposing a new feature/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants